feat(compiler): add @strictExtends to require union variants to nominally extend the base type - #11780
feat(compiler): add @strictExtends to require union variants to nominally extend the base type#11780JoshLove-msft wants to merge 11 commits into
Conversation
A named union can now declare a base type with `extends`. Every variant must be assignable to that base type, and the resolved type is exposed on the type graph as `Union.baseType` so emitters can represent the union with a polymorphic base type in languages without native unions. `extends` on a union is purely a constraint: it doesn't create any inheritance relationship, the base type doesn't become a variant, it doesn't make the union extensible and it has no interaction with `@discriminator`. Fixes microsoft#2737 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4
…end the base type The `extends` clause of a union is a structural constraint: any variant with a compatible shape satisfies it. Emitters targeting languages without native unions represent such a union with a polymorphic base type, which requires each variant to actually derive from the base type. `@strictExtends` turns that into a compile time error. It only adds a constraint when the base type is a model, since assignability between scalars is already nominal. A variant that is itself a union satisfies the constraint when all of its own variants do, so unions can still be composed. Implements the opt-in decorator proposed in microsoft#2737 and tracked by microsoft#3900. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4
…rsive types `isTypeAssignableToInternal` created a brand new relation cache for every nested call instead of forwarding the one it was given, so the "in progress" entry seeded by `areModelsRelated` only survived a single level and mutually recursive models recursed forever. Unions were never seeded at all, so any union reaching itself did the same. Forwarding the cache alone is not enough: the cache stored only the `Related` result and dropped the errors, and `areModelsRelated` turns a result with no errors back into `Related.true`. The cache now stores the errors alongside the result. A purely cyclic union describes an empty set of values, so it is vacuously assignable to anything, and the dual seed is used on the target side where being assignable to a union only requires one variant to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4
Address the review findings on the first `@strictExtends` implementation. - Validate from `onTargetFinish` instead of during decorator application so a decorator applied later cannot invalidate the guarantee. - Report an error when the base type is not a model, a scalar or an enum instead of silently accepting every variant. - Handle scalars, string/number/boolean literals and enum members instead of only models, so a literal that is structurally assignable to a custom scalar is rejected. - Treat `never`, an empty union and a union cycle as satisfying the constraint so composition is closed: a union that `@strictExtends` accepts on its own is always usable as a variant of another one. - Memoize the walk, which was exponential on a union graph reachable through many paths. Results that relied on short circuiting a cycle are not memoized because they are only valid for that walk. - Name the offending leaf and point the diagnostic at the variant type expression instead of the whole variant. - Stay silent when the base type failed to resolve, and when the variant is not even assignable to the base type, since the checker already reported both. - Export `$strictExtends` from the package entry point like the other built in decorators. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4
commit: |
|
All changed packages have been documented.
Show changes
|
Follow up on a second adversarial review pass. - Validate from `onGraphFinish` instead of `onTargetFinish`. A union that is part of a cycle is still being built when the decorated union finishes, so validating earlier made the result depend on declaration order: moving the offending variant before the back edge changed an accepted program into a rejected one. - Stop assuming a variant that isn't assignable to the base type was already reported by the checker. The checker validates the variants a union was declared with, so a decorator that replaces one afterwards would bypass the validation entirely. A variant that satisfies neither constraint now reports both. - Replace the memoized walk with a plain reachability walk. A union carries no value of its own, so the question is only whether anything offending is reachable through its variants, which makes cycles fall out naturally. The previous cycle guard disabled memoization for every ancestor of a cycle and was exponential on a cyclic graph reachable through many paths: 8s at depth 24, instant now at depth 30. - Validate a union once even when the decorator is applied more than once, directly or through `@@strictExtends`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4
|
You can try these changes here
|
Graph finish validators are only drained once, at the end of the checking stage, so a `@strictExtends` union created after that - for example a clone a mutator produces during `$onValidate` - registered a validator that was never run, and was silently never validated. Keep deferring to graph finish while the program is being checked, which is what makes the result independent of declaration order for cyclic unions, and validate at target finish otherwise. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4
…hecking `postCheckValidators` is drained once, at the end of `checkProgram`, so a decorator applied to a type created after that - a clone a mutator produced during `$onValidate` for example - registered an `onGraphFinish` validator that was never run. The checker now runs those validators as soon as the type is finished, since there is no graph finish left to wait for. `@strictExtends` relies on this: it validates at graph finish so the result doesn't depend on the declaration order of a cyclic union, and a post-check clone would otherwise silently never be validated. Using `program.currentStage` to detect this in the decorator instead is not enough: the stage stays `"checking"` when the checker reported errors, which is exactly the state a language server program is left in. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4
…heck types Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4
Important
Draft / design proposal. This is stacked on #11771 and #11779, so the diff below contains both. Only the
@strictExtendscommits are new here. Please review commit by commit.This addresses #3900 (
design:needed, no accepted design yet), so the shape below is a proposal, not a finished decision. Opening it as a draft to have something concrete to discuss.Fixes #3900
Why
#2737 / #11771 added
union Foo extends Base { ... }, and by design that clause is a structural constraint: any variant whose shape is assignable toBasesatisfies it.Emitters targeting languages without native unions (C#, Java, Go, ...) represent such a union as a polymorphic base type, and that representation only works if every variant actually derives from the base. A variant that merely happens to have the same shape cannot be emitted as a subclass.
@strictExtendsis an opt in decorator that turns that requirement into a compile time error. It changes nothing for existing code and nothing about the default behaviour of #11771.Semantics
@strictExtends?string/numeric/booleanliteralnever, an empty union, a union cycleThe base type must be a model, a scalar or an enum. Those are the only types that can be explicitly extended, so
@strictExtendson a union whose base is another union, a tuple orunknownis an error rather than a silent no-op.A union carries no value of its own, so the check is really a reachability question: is anything offending reachable from the variant through union edges? That framing is what makes cycles, empty unions and
neverfall out for free, and it keeps composition closed - a union that satisfies@strictExtendson its own is always usable as a variant of another one.Implementation notes
onGraphFinish. Anything earlier is observably wrong: a union that is part of a cycle is still being built when the decorated union finishes, which made the outcome depend on the declaration order of the other union''s variants. It also means a decorator applied later on the same union cannot invalidate the guarantee. Both cases have tests.postCheckValidatorsis drained once, at the end ofcheckProgram, so anonGraphFinishvalidator registered for a type created after that (a clone a mutator produces during$onValidate, for example) was registered and never run. The checker now runs it as soon as the type is finished.@typespec/httpis the otheronGraphFinishconsumer and had the same latent hole. Detecting this in the decorator viaprogram.currentStageis not enough: the stage stays"checking"when the checker reported errors, which is exactly the state a language server program is left in.unassignableandstrict-extends-variant. Suppressing the second one required assuming the checker had already reported the first, which is not true once a decorator rewrites the variants.@@strictExtends, validates the union once.Open design questions
union Foo extends strict Base)? A decorator keeps feat(compiler): addextendsbase type clause for unions #11771 untouched and is easy to remove if the design lands differently.onGraphFinishis weaker than at graph finish. For a type created after checking it can only see the graph as it exists when that type is finished, so it cannot observe relationships attached later. That is disclosed onDecoratorValidatorCallbacks.onGraphFinish; giving it true graph-finish semantics would need an explicit "dynamic graph completed" operation, which felt out of scope here.isStrictExtends(program, union)) instead of each one re-deriving it? The state is tracked internally already.unassignable+strict-extends-varianton the same variant acceptable, or should the strict one be suppressed at the cost of the soundness hole described above?Validation
tsc -p tsconfig.build.json --noEmitclean@strictExtendstests@typespec/openapi32578 tests pass,@typespec/http21/21 files passpnpm lintclean, prettier cleanonTargetFinish, restoring the assignability suppression, removing the duplicate application guard, removing memoization and disabling reuse across cycles each make exactly the intended tests fail.Review history
Three adversarial review passes were run against this change. The first found 6 issues (silent bypass for non model base types, exponential walk, decorator ordering, composition not closed, missing export, diagnostic quality); the second found 3 more (declaration order dependence, the assignability suppression bypass, exponential behaviour on cyclic graphs) plus duplicate diagnostics on repeated application. The third found that
onGraphFinishvalidators registered after checking never ran, which is the checker fix above; the fourth found nothing further. All are fixed and covered by tests.--generated by Copilot